Skip to content

Conversation

@david-pl
Copy link
Collaborator

Closes #592 .

We are now mostly re-using the AddressAnalysis for walking the IR. Except for IfElse statements, where we need to take care when merging results from branches.
To do this, I had to make a small change to AddressAnalysis, but that shouldn't affect anything else.

Probably needs more tests, but basics seem to work quite well.

Also, we might need some wrapper around running this analysis to have a decent UX when trying to study the fidelity of a given program.

@weinbe58 I'm also not sure if I'm being to strict about expecting an AddressReg for qubtis: IList[Qubit] arguments.

@david-pl david-pl requested a review from weinbe58 November 26, 2025 16:09
@codecov
Copy link

codecov bot commented Nov 26, 2025

Codecov Report

❌ Patch coverage is 91.12150% with 19 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
src/bloqade/squin/analysis/fidelity/impls.py 86.17% 13 Missing ⚠️
src/bloqade/analysis/fidelity/impls.py 92.10% 3 Missing ⚠️
src/bloqade/qasm2/dialects/noise/fidelity.py 88.88% 3 Missing ⚠️

📢 Thoughts on this report? Let us know!

@github-actions
Copy link
Contributor

github-actions bot commented Nov 26, 2025

☂️ Python Coverage

current status: ✅

Overall Coverage

Lines Covered Coverage Threshold Status
10391 9132 88% 0% 🟢

New Files

File Coverage Status
src/bloqade/analysis/fidelity/impls.py 92% 🟢
src/bloqade/squin/analysis/fidelity/_init_.py 100% 🟢
src/bloqade/squin/analysis/fidelity/impls.py 86% 🟢
TOTAL 93% 🟢

Modified Files

File Coverage Status
src/bloqade/analysis/address/analysis.py 90% 🟢
src/bloqade/analysis/fidelity/_init_.py 100% 🟢
src/bloqade/analysis/fidelity/analysis.py 100% 🟢
src/bloqade/qasm2/dialects/noise/fidelity.py 92% 🟢
src/bloqade/squin/_init_.py 100% 🟢
TOTAL 96% 🟢

updated for commit: 053e798 by action🐍

Copy link
Member

@weinbe58 weinbe58 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall looks good, just one comment on the implementation of IfElse. Also I would add a test to make sure that you execute only one branch if the branch value is known at compile time.

@weinbe58
Copy link
Member

@weinbe58 I'm also not sure if I'm being to strict about expecting an AddressReg for qubtis: IList[Qubit] arguments.

It depends on the situation, the short has is AddressReg is an IList of all qubits with known addresses PartialIList if some of the qubit in the list have a known address and some have UnknownQubit or if the size of the array is known at compile time and finally UnknownReg you know its an IList of qubit values but you have no idea the size of the list.

@david-pl
Copy link
Collaborator Author

@weinbe58 Thanks for the review. The IfElse impls was somewhat left over from a previous version. It was also missing the return values. I fixed that, added the constant condition evaluation and a test.

It depends on the situation, the short has is AddressReg is an IList of all qubits with known addresses PartialIList if some of the qubit in the list have a known address and some have UnknownQubit or if the size of the array is known at compile time and finally UnknownReg you know its an IList of qubit values but you have no idea the size of the list.

I suppose it's fine then. Basically, the fidelity analysis will give up whenever there is any qubit without a known address. Technically, we could update the fidelity at least for the known qubits, but something's bound to go wrong in this case anyway since at least for one qubit the fidelity will be off.

@david-pl david-pl requested a review from weinbe58 November 27, 2025 09:10
@weinbe58 weinbe58 self-assigned this Dec 3, 2025
Copy link
Member

@weinbe58 weinbe58 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@weinbe58 weinbe58 merged commit 0958f24 into main Dec 3, 2025
11 of 12 checks passed
@weinbe58 weinbe58 deleted the david/592-3-fidelity-analysis branch December 3, 2025 15:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Proper implementation of the fidelity analysis

3 participants